fix: harden IPC limits and secure file handling#377
Open
SUJALMU2004 wants to merge 1 commit into
Open
Conversation
- Increased Unix socket server limit to 16MB to allow agents to inject large JS/HTML payloads without crashing. - Maintained Windows TCP loopback at 64KB limit to prevent pre-auth local connection DoS. - Switched socket unlink to use EAFP (try/except OSError) instead of os.path.exists to fix a crash caused by broken symlinks. - Added a safe_open_write helper with O_NOFOLLOW and applied it to PID/LOG files to patch a CWE-61 symlink overwrite vulnerability in shared /tmp directories.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces targeted, non-destructive hardening improvements to the daemon's IPC and file handling architecture.
During load testing and security review of the local daemon setup, I identified a functional limitation with large CDP payloads, a potential ToCToU socket creation race, and a CWE-61 symlink vulnerability in the default
/tmplogging mechanism.1. Fix IPC Payload Limit (Architecture & Functional)
The Root Cause:
asyncio.start_unix_serverlimits thereader.readline()buffer to 64KB by default. If an agent injects a large JavaScript library viahelpers.js()or sends a large DOM CDP payload, the daemon silently terminates the connection with aLimitOverrunError.The Fix:
I explicitly passed
limit=1024 * 1024 * 16(16MB) to the Unix socket initialization to natively support arbitrarily large AI-generated scripts. (Note: Windows TCP loopback is intentionally left at 64KB to prevent unauthenticated local clients from forcing large pre-auth memory allocations).2. Prevent Unix Socket Denial of Service (Robustness)
The Root Cause:
The daemon cleans up stale sockets using
if os.path.exists(path): os.unlink(path). Becauseos.path.exists()resolves symlinks, it returnsFalseif the path is a broken symlink. If a broken symlink exists at the socket path, theunlinkis bypassed, and thebind()call fatally crashes withAddress already in use.The Fix:
Refactored socket cleanup to use the standard, race-free EAFP pattern:
try: os.unlink(path) except OSError: pass.3. Patch Log/PID Symlink Vulnerability (Security - CWE-61)
The Root Cause:
daemon.pycreated theLOGandPIDfiles directly in/tmp. Because/tmpis a world-writable shared directory on POSIX systems, a malicious local user could execute a symlink attack (CWE-61) by pre-creating/tmp/bu-default.logas a symlink pointing to another user's sensitive file (e.g.,~/.bashrc). The daemon would blindly follow the symlink and overwrite the target file.The Fix:
Introduced a
safe_open_write()helper utilizing the low-levelos.opensyscall with theos.O_NOFOLLOWflag to strictly reject symlink traversal at the kernel level (falling back gracefully to standard behavior on Windows).Summary by cubic
Hardened the daemon’s IPC and file handling for stability and security. Supports large Unix socket payloads and blocks symlink-based file overwrites.
Bug Fixes
Security
safe_open_write()usingos.O_NOFOLLOWand 0600 perms; applied toLOGandPIDin/tmpto prevent CWE-61 symlink overwrites.Written for commit 1e59cc8. Summary will update on new commits. Review in cubic